Skip to content

Conversation

@1luik
Copy link

@1luik 1luik commented Dec 17, 2025

修复#3103 ,但是获取不到minecraft包还是编译不了

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


1luik seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@1luik
Copy link
Author

1luik commented Dec 17, 2025

图片

@1luik
Copy link
Author

1luik commented Dec 17, 2025

我是使用的idea登录github提交的

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

import javax.tools.JavaFileObject;
import java.io.*;
import java.net.URI;
import java.nio.charset.Charset;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unused import java.nio.charset.Charset is imported but never used in this class.

Suggested change
import java.nio.charset.Charset;

Copilot uses AI. Check for mistakes.
import java.util.zip.ZipException;
import java.util.zip.ZipFile;

public class GetZipFile extends ZipFile {
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class name GetZipFile is poorly named as it uses a verb ("Get") for a class that represents an extended ZipFile. A better name would be something like CustomZipFile, ExtendedZipFile, or StreamableZipFile to better describe what the class represents.

Copilot uses AI. Check for mistakes.
import java.net.URI;
import java.net.URL;
import java.net.URLDecoder;
import java.net.*;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using wildcard import import java.net.*; reduces code clarity. It's better to explicitly import only the classes that are actually used (URLConnection, JarURLConnection, URI, URL, URLDecoder) so readers can easily see the dependencies.

Suggested change
import java.net.*;
import java.net.JarURLConnection;
import java.net.URI;
import java.net.URL;
import java.net.URLConnection;
import java.net.URLDecoder;

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,113 @@
package com.taobao.arthas.compiler;

import javax.lang.model.element.Modifier;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unused import javax.lang.model.element.Modifier is imported but never used in this class.

Copilot uses AI. Check for mistakes.
package com.taobao.arthas.compiler;

import javax.lang.model.element.Modifier;
import javax.lang.model.element.NestingKind;
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unused import javax.lang.model.element.NestingKind is imported but never used in this class.

Copilot uses AI. Check for mistakes.
}
}
}
}catch (SecurityException e) {
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a URISyntaxException handler. The new URI(jarUri) call on line 132 can throw URISyntaxException, but the catch block on line 171 only catches SecurityException. This means URISyntaxException will be caught by the outer catch block on line 175, which may not provide the appropriate error message for URI syntax issues.

Suggested change
}catch (SecurityException e) {
} catch (URISyntaxException e) {
throw new RuntimeException("Invalid URI syntax for jarUri: " + jarUri, e);
} catch (SecurityException e) {

Copilot uses AI. Check for mistakes.
Comment on lines +129 to 174
}else {
//是否可以在硬盘上找到对应的文件
try {
URI uri1 = new URI(jarUri);
// 获取 authority(host:port) + path + query + fragment
StringBuilder sb = new StringBuilder();
if (uri1.getAuthority() != null) {
sb.append(uri1.getAuthority());
}
if (uri1.getPath() != null) {
sb.append(uri1.getPath());
}
if (uri1.getQuery() != null) {
sb.append('?').append(uri1.getQuery());
}
if (uri1.getFragment() != null) {
sb.append('#').append(uri1.getFragment());
}

Enumeration<JarEntry> entryEnum = jarConn.getJarFile().entries();
while (entryEnum.hasMoreElements()) {
JarEntry jarEntry = entryEnum.nextElement();
String name = jarEntry.getName();
if (name.startsWith(rootEntryName) && name.indexOf('/', rootEnd) == -1 && name.endsWith(CLASS_FILE_EXTENSION)) {
URI uri = URI.create(jarUri + "!/" + name);
String binaryName = name.replaceAll("/", ".");
binaryName = binaryName.replaceAll(CLASS_FILE_EXTENSION + "$", "");
File file = new File(sb.toString());
if (file.exists()) {
//可能是文件或目录
if (file.isDirectory()) {
result.addAll(processDir(packageName, file));
} else {
try {
GetZipFile zipFile = new GetZipFile(file);
zipFile.stream().forEach(e -> {
String name = e.getName();
if (name.startsWith(packageName) && name.endsWith(CLASS_FILE_EXTENSION)) {
URI uri = URI.create(jarUri + "!/" + name);
String binaryName = name.replaceAll("/", ".");
binaryName = binaryName.replaceAll(CLASS_FILE_EXTENSION + "$", "");

result.add(new CustomJavaFileObject(binaryName, uri));
result.add(zipFile.new ZipJavaFileObject(binaryName, uri, e));
}
});
}catch (ZipException e) {
// ignore
}
}
}
}catch (SecurityException e) {
throw new RuntimeException("Failed to open " + packageFolderURL + " as a jar file", e);
}
}
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new fallback logic (lines 129-174) for handling non-JarURLConnection cases lacks test coverage. Given the complexity of the URI parsing and file system access logic, and the fact that this addresses a specific issue with Minecraft Forge's custom URL handlers, adding test cases for this code path would help ensure it works correctly and prevent regressions.

Copilot uses AI. Check for mistakes.
Comment on lines +132 to +145
URI uri1 = new URI(jarUri);
// 获取 authority(host:port) + path + query + fragment
StringBuilder sb = new StringBuilder();
if (uri1.getAuthority() != null) {
sb.append(uri1.getAuthority());
}
if (uri1.getPath() != null) {
sb.append(uri1.getPath());
}
if (uri1.getQuery() != null) {
sb.append('?').append(uri1.getQuery());
}
if (uri1.getFragment() != null) {
sb.append('#').append(uri1.getFragment());
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name uri1 is not descriptive. Consider using a more meaningful name like parsedJarUri or jarFileUri to clearly indicate what this URI represents.

Suggested change
URI uri1 = new URI(jarUri);
// 获取 authority(host:port) + path + query + fragment
StringBuilder sb = new StringBuilder();
if (uri1.getAuthority() != null) {
sb.append(uri1.getAuthority());
}
if (uri1.getPath() != null) {
sb.append(uri1.getPath());
}
if (uri1.getQuery() != null) {
sb.append('?').append(uri1.getQuery());
}
if (uri1.getFragment() != null) {
sb.append('#').append(uri1.getFragment());
URI parsedJarUri = new URI(jarUri);
// 获取 authority(host:port) + path + query + fragment
StringBuilder sb = new StringBuilder();
if (parsedJarUri.getAuthority() != null) {
sb.append(parsedJarUri.getAuthority());
}
if (parsedJarUri.getPath() != null) {
sb.append(parsedJarUri.getPath());
}
if (parsedJarUri.getQuery() != null) {
sb.append('?').append(parsedJarUri.getQuery());
}
if (parsedJarUri.getFragment() != null) {
sb.append('#').append(parsedJarUri.getFragment());

Copilot uses AI. Check for mistakes.
Comment on lines +32 to +37
Enumeration<? extends ZipEntry> entries = super.entries();
List<ZipEntry> entryList = new ArrayList<>();
while (entries.hasMoreElements()) {
entryList.add(entries.nextElement());
}
return entryList.stream();
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stream() method loads all ZipEntry elements into an ArrayList before converting to a stream, which is inefficient for large zip files. This creates unnecessary memory overhead. Consider using Collections.list(entries).stream() or implementing a custom Spliterator for better performance and memory efficiency.

Suggested change
Enumeration<? extends ZipEntry> entries = super.entries();
List<ZipEntry> entryList = new ArrayList<>();
while (entries.hasMoreElements()) {
entryList.add(entries.nextElement());
}
return entryList.stream();
return super.stream().map(e -> (ZipEntry) e);

Copilot uses AI. Check for mistakes.
Comment on lines +155 to +165
GetZipFile zipFile = new GetZipFile(file);
zipFile.stream().forEach(e -> {
String name = e.getName();
if (name.startsWith(packageName) && name.endsWith(CLASS_FILE_EXTENSION)) {
URI uri = URI.create(jarUri + "!/" + name);
String binaryName = name.replaceAll("/", ".");
binaryName = binaryName.replaceAll(CLASS_FILE_EXTENSION + "$", "");

result.add(new CustomJavaFileObject(binaryName, uri));
result.add(zipFile.new ZipJavaFileObject(binaryName, uri, e));
}
});
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GetZipFile resource is not properly closed. The ZipFile is created but never closed, which can lead to resource leaks and file handle exhaustion. Consider using try-with-resources or ensuring the file is closed after processing.

Copilot uses AI. Check for mistakes.
@1luik
Copy link
Author

1luik commented Jan 14, 2026

6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants